StaticFetcher() Implementation + from_pdb() refactor - #5436

Checking whether a requested file is present in the registry should not require scanning the entire cache directory. That is error-prone since unrelated cache files can also cause false failures; it also adds unnecessary I/O.

check_registry() might be a misleading name. This method is meant to return all files not in the self.cache_path per the docstring

Return paths relative to :attr:`cache_path` for cache files that are
missing from the registry.

I originally made it when I was refactoring the fetch() method where it would toss error to prevent unintentional update of the registry (which would be a security issue)

        if LOAD_FROM_CACHE:
            registry_dictionary = self.read_registry(db_path)
            missing_files_list = self.check_registry(
                db_path, files=list(requested_files)
            )

            if len(missing_files_list) != 0:
                MISSING_FILES = True

        if MISSING_FILES and not APPEND_DATABASE:
            raise ValueError(
                "fetch() is requesting files not found in the registry. "
                + f"The missing files are {missing_files_list}. "
                + "To fix this, please set append_db=True to append the "
                + "registry."
            )

This method could be renamed because it's not really checking like return a True or False

I added comments to PR #5457 as I hadn't realized that you're first introducin StaticFetcher here. Please see #5457 (review)

verbose is currently not passed along properly. Pooch only reads the progressbar argument of Pooch.fetch() when no downloader is given (if downloader is None: downloader = choose_downloader(url, progressbar=progressbar)), and StaticFetcher always supplies one.

from_PDB(progressbar=True) only works because the unknown progressbar kwarg leaks through **kwargs into HTTPDownloader(progressbar=...).

Explicitly forward progressbar=verbose into the downloader constructor inside _set_downloader, having from_PDB call verbose=progressbar, and adding a test that asserts the downloader receives it.

This does change the current return type compared to develop which always returned a list of Path items.

I think we should have single item / string in -> Path out, sequence in -> list[Path] out, otherwise people will constantly have to unpack the first item in the list to use it.

A scenario worth thinking through: the registry stores the hash of whatever was first downloaded. If the upstream file later changes (PDB entries do get revised) and the cached copy has been evicted, pooch raises a hash mismatch. Before that, retry_if_failed retries the download twice with sleeps, which can never fix a hash mismatch. The only recovery is deleting hashes.txt, and pooch's error message doesn't say so.

Related: for from_PDB, which always passes append_db=True, the registry offers no protection against a bad server because the hash is derived from the download itself. It only detects local cache corruption. That's fine, but the docs should say so.

Suggestions: catch pooch's hash ValueError and re-raise with a recovery hint, and decide explicitly whether from_PDB needs a registry at all.

The docstring promises the pooch registry format, but two valid pooch registry files fail here:

• A blank line (e.g. trailing newline after a hand edit) raises IndexError on line.lstrip()[0].
• The optional third URL column (fname hash url) raises ValueError: too many values to unpack on the next line.

Skip empty lines (if not line.strip() or line.lstrip().startswith("#")) and take only the first two fields, or delegate to pooch's own load_registry.

ignore doesn't behave as documented. The docstring and example say entries are relative to cache_path (ignore=["file2.txt"]), but this compares against absolute Path objects from rglob, so a relative entry never matches. I confirmed ignore=["b.txt"] leaves b.txt in the result while ignore=[cache_path / "b.txt"] removes it. The test only passes absolute paths, which is why it passes.

Resolve ignore entries against cache_path the same way files is handled a few lines up.

Return and argument type hinting

type hinting

type hinting

type hitnig

type hinting